Skip to content

[AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration - #2448

Merged
Tartarus0zm merged 3 commits into
apache:masterfrom
weiqingy:AURON-1863-converter
Aug 11, 2026
Merged

[AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration#2448
Tartarus0zm merged 3 commits into
apache:masterfrom
weiqingy:AURON-1863-converter

Conversation

@weiqingy

@weiqingy weiqingy commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #1863. Not Closes, because the issue description also covers the 0-input form, which this PR does not implement.

This is the Flink Java side of UNIX_TIMESTAMP. The native function it calls merged in #2409. Together they cover the 1-input and 2-input forms from the issue description.

The 0-input form is still outstanding. It is a different function rather than a missing branch: Flink binds the niladic form to DateTimeUtils.unixTimestamp(), which reads the clock per record and parses nothing, so it needs its own design pass. The converter rejects it explicitly and falls back to Flink's engine, and #1863 stays open to track it.

Rationale for this change

This completes native support for Flink's UNIX_TIMESTAMP by wiring the Flink Calc converter to emit the native function added in #2409. Without it the native function is unreachable, and any Calc containing UNIX_TIMESTAMP falls back to Flink's engine for the whole Calc.

What changes are included in this PR?

The converter recognizes UNIX_TIMESTAMP and lowers the 1-argument and 2-argument forms to the native Flink_UnixTimestamp node.

UNIX_TIMESTAMP resolves to SqlKind.OTHER_FUNCTION, so it is matched by reference identity on the operator before the supported-kinds switch, the same way TRY_CAST is handled.

A format scanner translates the supported subset of Java date-format letters (yyyy MM dd HH mm ss and literals) to the native format. Anything outside that subset falls back: other pattern letters, unsupported run-lengths, a non-literal format argument, the 0-argument form, and a numeric field adjacent to another numeric field where the run-length would not survive translation (for example yyyyMd). Falling back keeps results correct rather than risking a silent divergence.

The session time zone is resolved at plan time and passed into the node. This required completing the config threading in the standalone-Calc path, which passed a persisted config that does not carry table.local-time-zone. That gap had no effect until now: UNIX_TIMESTAMP is the first time-zone-sensitive expression the converter supports, and the earlier ones (arithmetic, comparison, logical, cast) never read the session zone. The effective node config is threaded through instead, so the configured zone reaches the native evaluation.

Are there any user-facing changes?

Yes. UNIX_TIMESTAMP(string) and UNIX_TIMESTAMP(string, format) now execute on the native engine when the format is a supported literal pattern. Unsupported patterns and the 0-argument form continue to run on Flink's engine, with the same results as before.

How was this patch tested?

Unit tests for the scanner (accept/reject, quote escaping, the adjacency rule), the converter (node shape, format translation, time-zone propagation), and the operator-identity invariant. Fallback tests assert that unsupported inputs actually fall back rather than silently producing a native plan.

An end-to-end ITCase runs UNIX_TIMESTAMP(ts) with a non-UTC session zone and confirms the native result matches the expected epoch values. The executed native plan shows the function and the resolved zone, confirming the query runs natively rather than falling back.

160 tests pass in auron-flink-planner on the rebased branch, with spotless clean and 0 checkstyle violations.

Was this patch authored or co-authored using generative AI tooling?

  • Yes
  • No

Generated-by: Claude Code (Claude Opus 5)

…tegration

Wire Flink's UNIX_TIMESTAMP (1-arg and 2-arg forms) to the native
Flink_UnixTimestamp function. The converter matches the operator by
reference identity (as TRY_CAST does), translates the supported subset of
Java date-format patterns to the native format, resolves the session
time zone at plan time, and builds the native scalar-function node.

Formats outside the supported subset, non-literal format arguments, the
0-argument form, and format patterns whose lenient parse cannot be
represented after translation all fall back to Flink's engine. The
0-argument form is rejected explicitly at both the gate and the builder,
so a call that parses nothing and reads the wall clock per record can
never reach a native function that expects a value operand to size its
output against.

Also propagate the effective node config (carrying table.local-time-zone)
into the standalone-Calc converter path; the persisted config it previously
used does not carry the session time zone, which would silently default the
native evaluation to UTC.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds planner-side support for Flink UNIX_TIMESTAMP by lowering supported call shapes to the native Flink_UnixTimestamp ext scalar function, including plan-time translation of Java SimpleDateFormat patterns to the native strftime-like format and propagation of the effective session time zone into the emitted node. It also fixes config-threading so session options (notably table.local-time-zone) reach native conversion in the standalone Calc path.

Changes:

  • Teach RexCallConverter to recognize UNIX_TIMESTAMP by operator identity and emit a Flink_UnixTimestamp ext-function node with args [value, chronoFormat, zoneId].
  • Introduce a Java pattern translator (FlinkDateTimeFormatConverter) with a conservative allowlist + adjacency rule to decide native eligibility vs fallback.
  • Thread the effective ExecNodeConfig (not the persisted node config) into native Calc plan building to preserve session configuration such as time zone, and add targeted unit + IT coverage.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
auron-flink-extension/auron-flink-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/stream/StreamExecCalcTest.java Adds planner-level tests for 0-arg fallback and time-zone propagation into the native plan.
auron-flink-extension/auron-flink-planner/src/test/java/org/apache/auron/flink/table/runtime/AuronFlinkCalcITCase.java Adds an end-to-end IT case validating native execution + non-UTC session zone behavior.
auron-flink-extension/auron-flink-planner/src/test/java/org/apache/auron/flink/table/planner/converter/UnixTimestampOperatorIdentityTest.java Pins Flink operator identity invariant relied on by the converter dispatch.
auron-flink-extension/auron-flink-planner/src/test/java/org/apache/auron/flink/table/planner/converter/RexCallConverterTest.java Adds unit tests for node shape, format translation, time-zone literal propagation, and fallback gates.
auron-flink-extension/auron-flink-planner/src/test/java/org/apache/auron/flink/table/planner/converter/FlinkDateTimeFormatConverterTest.java Adds focused tests for accepted/rejected patterns, quote escaping, and adjacency hazard rule.
auron-flink-extension/auron-flink-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/stream/StreamExecCalc.java Uses the effective ExecNodeConfig when seeding the native converter context to preserve session settings.
auron-flink-extension/auron-flink-planner/src/main/java/org/apache/auron/flink/table/planner/converter/RexLiteralConverter.java Adds a helper to encode plan-time string constants into Arrow IPC literals for native arguments.
auron-flink-extension/auron-flink-planner/src/main/java/org/apache/auron/flink/table/planner/converter/RexCallConverter.java Implements UNIX_TIMESTAMP support (arity gating, format translation, zone resolution, ext-function emission).
auron-flink-extension/auron-flink-planner/src/main/java/org/apache/auron/flink/table/planner/converter/FlinkNodeConverterUtils.java Adds a helper to build ext scalar function nodes routed via AuronExtFunctions.
auron-flink-extension/auron-flink-planner/src/main/java/org/apache/auron/flink/table/planner/converter/FlinkDateTimeFormatConverter.java New Java SimpleDateFormat → native format translator with strict fallback behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +125 to +133
public static PhysicalExprNode stringLiteral(String value) {
RowType rowType = RowType.of(new VarCharType(VarCharType.MAX_LENGTH));
try (BufferAllocator allocator =
FlinkArrowUtils.getRootAllocator().newChildAllocator("literal", 0, Long.MAX_VALUE);
VectorSchemaRoot root = VectorSchemaRoot.create(FlinkArrowUtils.toArrowSchema(rowType), allocator)) {

GenericRowData rowData = new GenericRowData(1);
rowData.setField(0, StringData.fromString(value));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a requireNonNull in cccb1acb, though for a different reason than the one given here.

Null cannot reach this method today. It has two call sites, both in RexCallConverter.buildUnixTimestamp: one passes chronoFormat, which comes out of translate(...).orElseThrow(...) and so is non-null by construction, and the other passes zone.getId(), which ZoneId never returns null from.

On the diagnosis being opaque: conversion failures are already caught. FlinkNodeConverterFactory.convertRexNode wraps the convert call in catch (Exception e) and logs RexNode conversion failed for {} with the full stack trace, then returns empty so the Calc falls back to Flink. So an NPE here would surface as a logged stack trace plus a query that still returns correct results on Flink's engine, not an opaque failure.

What I did take from this: the method is public API introduced by this PR, and a null value would mean the caller failed to resolve a plan-time constant. Encoding that as a NULL literal would ship a silently wrong argument to the native side, so rejecting it at the boundary is the right contract to state. Guard plus javadoc plus testStringLiteralRejectsNull.

Comment on lines +70 to +72
public static Optional<String> translate(String javaPattern) {
List<Token> tokens = scan(javaPattern);
if (tokens == null) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a requireNonNull in cccb1acb, but deliberately not the Optional.empty() behavior suggested here.

Optional.empty() is this converter's signal that the user wrote a pattern outside the native surface, and the whole Calc should fall back to Flink. That is a normal user-facing outcome. A null pattern is not that: it would mean our own converter never resolved a format, which is a plumbing bug. Mapping it to empty() would route the bug into the same silent fallback and hide it, so it fails fast instead. The same split already exists elsewhere in this path, where isSupported returns false for user-facing cases and buildUnixTimestamp throws IllegalArgumentException for plumbing bugs.

Null also cannot reach it today. The gate at isUnixTimestampSupported reads javaFormat != null && FlinkDateTimeFormatConverter.translate(javaFormat).isPresent(), and && short-circuits. The other call site is inside the private buildUnixTimestamp, which the factory only reaches after isSupported returned true.

Covered by testNullPatternRejectedRatherThanReportedUntranslatable.

@weiqingy

weiqingy commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Hi @Tartarus0zm, could you please help review this PR when you get a chance? Thanks!

…lic entry points

stringLiteral and translate both encode plan-time constants the caller has
already resolved. A null argument means the caller never resolved one, which is
a plumbing bug rather than an unsupported expression.

translate rejects null instead of returning Optional.empty(): empty means the
user wrote a pattern outside the native surface and the Calc should fall back,
so reusing it for null would route a caller bug into the same silent fallback
and hide it.
Copilot AI review requested due to automatic review settings August 5, 2026 05:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

auron-flink-extension/auron-flink-planner/src/main/java/org/apache/auron/flink/table/planner/converter/RexCallConverter.java:473

  • buildUnixTimestamp assumes the 2nd operand is a non-null RexLiteral string. While the factory gates this today, calling convert(...) directly (as some tests already do for invalid arity) would currently throw ClassCastException / NullPointerException rather than the documented IllegalArgumentException. Adding explicit validation here makes failure mode deterministic and matches the method contract.
        String javaFormat = operands.size() > 1
                ? ((RexLiteral) operands.get(1)).getValueAs(String.class)
                : DEFAULT_UNIX_TIMESTAMP_FORMAT;
        String chronoFormat = FlinkDateTimeFormatConverter.translate(javaFormat)
                .orElseThrow(() -> new IllegalArgumentException("Unsupported UNIX_TIMESTAMP format: " + javaFormat));

@Tartarus0zm Tartarus0zm linked an issue Aug 5, 2026 that may be closed by this pull request
@Tartarus0zm
Tartarus0zm self-requested a review August 5, 2026 06:37

@Tartarus0zm Tartarus0zm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, just need more test cases

* deterministic and to exercise timezone propagation into the native plan. */
@Test
public void testUnixTimestamp() {
tableEnvironment.getConfig().setLocalTimeZone(ZoneId.of("Asia/Shanghai"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config table.local-time-zone supports multiple formats, such as GMT-08:00, I'd suggest adding more test cases for this.

@weiqingy weiqingy Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review.

The converter resolves the session zone at plan time and hands the id to the native function, which only knows IANA zone names. A fixed offset isn't one, so it errors during execution:

Flink_UnixTimestamp: invalid timezone GMT-08:00

By that point the plan is already native and the Calc operator has no runtime fallback, so the task dies. It also doesn't need an explicit SET. The default resolves to the JVM's default zone, so a TaskManager running with TZ=GMT-08:00 hits it with no config at all.

Fixed by checking the zone at plan time. If native can't resolve it, the Calc falls back to Flink and returns correct results, the same way the converter already handles formats it can't translate. That covers the GMT±HH:MM forms and the legacy SystemV/* ids.

Added the tests for: GMT-08:00 and UTC IT cases, plus unit tests on the gate.

Fixed-offset sessions fall back for now rather than running natively. Filed #2455 to follow up with native support for them so they stay on the native path.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weiqingy What's the rationale for not fixing this issue in this PR and instead opening a separate issue to follow up on it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I split it because this PR closes the correctness gap: fixed-offset zones now fall back to Flink instead of reaching the native call and failing. #2455 tracks the remaining acceleration gap.

Supporting those zones natively is not a big change, but it crosses the Java/Rust boundary and changes the same gate you're reviewing. parse_datetime and resolve_offset_secs currently take chrono_tz::Tz, so fixed offsets require a small generalization. I'd also want that path validated against SimpleDateFormat, as #2409's parser was.

I considered mapping offsets to the Etc/GMT names in Java to avoid the Rust change. POSIX reverses the sign there, though (GMT-08:00 maps to Etc/GMT+8), so a mistake could silently shift timestamps by twice the offset rather than merely leave them unaccelerated. Fractional offsets such as GMT+05:30 also have no Etc/GMT equivalent. #2455 has the details.

The plan-time gate would still remain for unsupported SystemV/* ids, so #2455 would only narrow it.

Since your earlier review was otherwise LGTM and the requested tests are now included, I thought keeping the Rust work separate would preserve a focused review scope. Happy to pull #2455 in if you'd rather see the native support completed in one PR.

…ely resolvable

The converter resolved the session time zone at plan time and passed
ZoneId.getId() to the native Flink_UnixTimestamp function, which resolves
zone ids by exact-match lookup in the IANA time zone database. Flink's
table.local-time-zone also accepts fixed-offset constructions such as
GMT-08:00, which name an offset rather than a region and have no entry in
that database. Those reached the native call and failed it:

  Flink_UnixTimestamp: invalid timezone GMT-08:00

The failure lands after the plan has been converted, where the Calc
operator has no run-time fallback, so the task died rather than degrading.
Besides an explicit configuration, this was reachable through
setLocalTimeZone(ZoneId.of("+08:00")), which normalizes to GMT+08:00, and
through the default value, which resolves to ZoneId.systemDefault().

Reject such zones at plan time instead, so the Calc falls back to Flink and
returns correct results, matching how an untranslatable format literal is
already handled. The legacy SystemV/* aliases need an explicit exclusion
because getAvailableZoneIds() carries them while the native lookup does not.

Pin the session zone in the converter test setup as well: those tests
inherited the machine's default zone, which the gate now reads.
Copilot AI review requested due to automatic review settings August 7, 2026 18:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

auron-flink-extension/auron-flink-planner/src/main/java/org/apache/auron/flink/table/planner/converter/RexCallConverter.java:437

  • isUnixTimestampSupported calls TableConfigUtils.getLocalTimeZone(...) without guarding against exceptions. FlinkNodeConverterFactory.convertRexNode does not catch exceptions thrown from isSupported, so a malformed table.local-time-zone (or any parsing error thrown by Flink) would fail plan conversion instead of cleanly falling back. Catch the exception here and return false so conversion remains fail-safe.
        ZoneId zone = TableConfigUtils.getLocalTimeZone(context.getTableConfig());
        if (!isNativelySupportedZone(zone.getId())) {
            return false;
        }

@Tartarus0zm Tartarus0zm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weiqingy thanks for your contribution! LGTM

@Tartarus0zm
Tartarus0zm merged commit 779b245 into apache:master Aug 11, 2026
123 checks passed
Tartarus0zm pushed a commit that referenced this pull request Aug 12, 2026
…#2465)

# Which issue does this PR close?

Part of #1863.

#1863 enumerates all three argument forms of `UNIX_TIMESTAMP`. The
1-argument and 2-argument string-parsing forms landed in #2409 and
#2448, and this PR adds the remaining 0-argument form. #1863 stays open
after this one merges, for the timezone configuration work tracked as
sub-issue #2455.

# Rationale for this change

`UNIX_TIMESTAMP()` with no arguments returns the current epoch seconds.
The converter rejected it, so any Calc containing it fell back to Flink
wholesale, taking every other expression in that Calc down with it.

Two things were believed to block it. Both turned out to be false, and I
stated one of them myself earlier in this issue.

**"A zero-argument ext function cannot size its output."** The
ext-function ABI drops the batch row count, so a 0-argument function
appears to have no way to know how many values to return. That is true
only for a function returning an `Array`. The length check in
`ScalarFunctionExpr::evaluate` sits entirely inside the
`ColumnarValue::Array` branch, so a function returning
`ColumnarValue::Scalar` never reaches it, and the projection broadcasts
the scalar to the batch width. No operand is needed, and no carrier
column either.

**"The 0-argument form needs the session time zone."** It does not. The
generated Flink operator holds a `timeZone` field, passes it to the
1-argument and 2-argument calls, and emits `result$1 =
DateTimeUtils.unixTimestamp()` with no argument at all. The result is
epoch seconds, which is zone-independent.

An approach was proposed on #1863 that works around the first premise:
pass any input column to the Rust function purely to convey the batch
length, ignore its data, and return that many timestamps. That would
have worked, and it was a sound answer to the constraint as I had
described it. This PR does not use it, because once the constraint turns
out not to exist the carrier column has no job left. The difference is
visible in the diff, so it is worth stating plainly: no column is
passed, the Rust function reads no length, and the row count comes from
the projection broadcasting a scalar. The observable behavior is the
same either way.

# What changes are included in this PR?

Three commits.

1. A native zero-argument ext function `Flink_UnixTimestampNow`
returning `ColumnarValue::Scalar(Int64)`. The clock is read as
`timestamp_millis() / 1000`, mirroring Flink's
`System.currentTimeMillis() / 1000` operator for operator.
2. The converter admits the 0-argument form and emits that function with
an empty operand list. The 0-argument arm sits **above** the session
time zone check, since a zone the native side cannot resolve is no
reason to fall back a query that never consults a zone. Class and method
javadoc are corrected: they previously described the 0-argument form as
falling back and described `UNIX_TIMESTAMP` as always mapping to
`Flink_UnixTimestamp` with `[value, chronoFormat, zoneId]`.
3. End-to-end coverage in `AuronFlinkCalcITCase`.

No protobuf change, no `Cargo.toml` change, no `pom.xml` change. It is a
separate native function rather than a fourth arity on
`Flink_UnixTimestamp`, whose contract is "parse this string with this
format in this zone" and which the 0-argument form shares none of.

# Are there any user-facing changes?

Yes, and one of them is a deliberate semantic divergence worth reviewing
on its own merits.

A query using `UNIX_TIMESTAMP()` that previously fell back to Flink now
runs natively.

**Flink evaluates the niladic form per record. This implementation reads
the clock once per call site per evaluation and broadcasts it across the
batch**, so rows in one batch share a timestamp, and that timestamp is
taken when the batch is evaluated rather than when each row arrived. Two
`UNIX_TIMESTAMP()` calls in one projection are read independently, which
matches Flink.

How large the skew can get depends on what closes the batch:

| Path | Bound |
|---|---|
| Calc with a declared `WATERMARK FOR` | about 205 ms, the default
`autoWatermarkInterval` |
| Calc with no watermark strategy | 8192 rows only, so `8192 / rate`
seconds. About 82 s at 100 rec/s |
| Fused Kafka plan | the native scan blocks until its buffer fills,
default 3000, with no time flush |

The unwatermarked case is the common shape for a processing-time query,
so this is not a corner case. It was discussed and settled on the issue
(#1863 (comment)):
per-batch is acceptable, since the zero-argument form is non-idempotent
anyway and what users generally want is to know roughly when a record
was processed. It is also the finest granularity among comparable
engines: StreamFusion stamps `PROCTIME()` once per operator compile,
Flink's own batch mode folds `CURRENT_TIMESTAMP` to a query-start
literal, DataFusion, ClickHouse and Velox are per query, DuckDB is per
transaction, and ClickHouse ships `nowInBlock()` as a documented
per-block clock. Flink also declares the niladic form only
`SqlMonotonicity.INCREASING`, which a per-batch constant satisfies.

Gating it conditionally on a declared watermark was considered and
rejected: the converter has no access to the ExecNode graph, the
predicate misses DataStream-level watermarks,
`auto-watermark-interval=0` and idle partitions, it keys native support
off an unrelated DDL clause, and it only shrinks the skew rather than
removing it.

# How was this patch tested?

Native unit tests for the scalar return and its broadcast to batch
width, and for the value being a plausible epoch second rather than
milliseconds.

Converter tests for the emitted node shape, and
`testUnixTimestampZeroArgSupportedWithFixedOffsetZone`, which exists to
catch the 0-argument arm being placed below the time zone check. It is a
real discriminator: moving the arm below the check fails that test and
no other. Two tests that asserted the form falls back are inverted.

An ITCase asserting one clock-bracketed row per input row and no
recorded fallback. Both assertions are load-bearing and neither subsumes
the other. A zero fallback count establishes that the Calc converted,
not that it ran: a native library holding no registry arm for the
function still converts at plan time and fails only during execution,
where nothing records a fallback, leaving the counter at zero and the
result set empty. The row count is what establishes the native plan
executed.

The ITCase was checked to be non-vacuous by running rather than by
inspection: reverting the converter gate fails it on the fallback count,
and running against a library without the registry arm fails it on the
row count.

Full module build green, 0 checkstyle violations, spotless clean.

# Was this patch authored or co-authored using generative AI tooling?
- [x] Yes
- [ ] No

`Generated-by: Claude Code (Claude Opus 5)`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Native Flink UNIX_TIMESTAMP Function

3 participants